Add error decoration for invalid libraries in the library list #2933
Conversation
Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
|
👋 A new build is available for this PR based on d146e0c. |
There was a problem hiding this comment.
@SanjulaGanepola Great PR, learned some new stuff about decorators. 😃
Almost there, see other comment.
Signed-off-by: Sanjula Ganepola <[email protected]>
|
@chrjorgensen Ready for review again! |
chrjorgensen
left a comment
chrjorgensen
left a comment
There was a problem hiding this comment.
One issue found - see comment.
And we should not ask for adding the new library to the library list, if it's already there... right?
Co-authored-by: Christian Jorgensen <[email protected]>
|
👋 A new build is available for this PR based on 8a280ae. |
Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
|
@chrjorgensen Good point, I fixed that! I also noticed that we previously always added a filter to the Object Browser even when the library fails to create. I changed this as well to only do so if it created successfully. Ready for review |
sebjulliand
left a comment
sebjulliand
left a comment
There was a problem hiding this comment.
Works as advertised and code looks good.
It can be merged, as far as I'm concerned.
@chrjorgensen, do you want to have one final look?
chrjorgensen
left a comment
chrjorgensen
left a comment
There was a problem hiding this comment.
@SanjulaGanepola If @sebjulliand is happy, then I'm happy. 😆
I didn't complete my test since I was on my Linux partition and didn't have access to my own IBM i partitions for library creation/deletion tests. @sebjulliand did the test, so I will approve and merge.
| } | ||
| }); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
@SanjulaGanepola
Not an issue, since the code is working - but this is a great example of the benefit of coding early exits: If you reversed the test for library created and moved this code right after the test and added a return, you could avoid indenting all the remaining code. Try googling "never nester"... 😉
There was a problem hiding this comment.
Good point, will keep that in mind for next time.
I also did just look it up and saw many interesting ways people stick to only three levels of depth. Very interesting!
Changes
libraryicon to libraries in the library listHow to test this PR
Checklist